Conversation
|
I think this PR uncovered some mistakes in the derivatives that weren't found because the scale_performance flag was false. |
| 'exceeds maximum flight idle fraction. Values for min and max ' | ||
| 'fraction will be flipped.' | ||
| f'EngineDeck <{self.name}>: Minimum flight idle fraction exceeds maximum ' | ||
| 'flight idle fraction. Values for min and max fraction will be flipped.' |
There was a problem hiding this comment.
Personally I would be in favor of removing this 'helpfulness' and throwing an error if a user inputs min fraction > max fraction rather than guessing these are the correct values entered for the wrong variables.
If a user entered a fuselage width greater than its length would we throw a warning and try and flip them? What about for Min and Max Mach numbers for a cruise segment, I'm sure OpenMDAO / Dymos will error if Min > Max in that case rather than assuming we can guess what the user meant?
| f'EngineDeck <{self.name}>: Conflicting values provided for ' | ||
| 'aircraft:engine:scale_factor and aircraft:engine:scaled_sls_thrust when ' | ||
| 'compared against aircraft:engine:reference_sls_thrust' |
There was a problem hiding this comment.
| f'EngineDeck <{self.name}>: Conflicting values provided for ' | |
| 'aircraft:engine:scale_factor and aircraft:engine:scaled_sls_thrust when ' | |
| 'compared against aircraft:engine:reference_sls_thrust' | |
| f'EngineDeck <{self.name}>: Conflicting values provided for ' | |
| 'aircraft:engine:scale_factor {scale_factor} and aircraft:engine:scaled_sls_thrust {scaled_thrust} when ' | |
| 'compared against aircraft:engine:reference_sls_thrust {ref_thrust}' |
Perhaps we can add in the values that break the check here so the user can easily find and change the discrepancy.
| warnings.warn( | ||
| f'EngineDeck <{self.name}>: aircraft:engine:scaled_sls_thrust and ' | ||
| 'product of aircraft:engine_scale_factor and ' | ||
| 'aircraft:engine:reference_sls_thrust are not an exact match but within ' | ||
| 'tolerance. Setting scaled thrust target to calculated value of ' | ||
| f'{target_thrust} lbf.' | ||
| ) |
There was a problem hiding this comment.
I think we should simplify this and go with the following logic:
If both SCALED_SLS_THRUST and SCALE_FACTOR are provided warn the user, ignore one and use the other (maybe sscale_factor should take precedence)?
This eliminates needing to do a tolerance check, and Aviary's logic is clear: it uses scale_factor to scale the engine. If scale factor is not provided it will be calculate based on SCALED_SLS_THRUST and reference thrust.
I think this warning as written will always trigger if thrust_provided = True, even if the the scaling is an exact match? If we want this warning then we should add it under an additional 'if' checking for within tolerance but not exact? Personally I think this is unecessary - if the provided scaling matches to within 1e-4 that's 0.01% and more than accurate enough to be ignored.
| @@ -1364,44 +1320,34 @@ def _set_reference_thrust(self): | |||
| # both scale factor and target thrust provided: | |||
There was a problem hiding this comment.
As described below I think we should simplify this block of code for if we have both.
I would throw a warning stating both have been provided and so scaled_sls_thrust will be ignored.
Then I would move to the 'scale_factor has been provided, no scaled_thrust provided' logic block so Aviary is consistent and remove the complexity of the tolerance and warnings etc.
3 participants
Summary
Removes unnecessary variable Aircraft.Engine.SCALE_PERFORMANCE
Related Issues
Backwards incompatibilities
None
New Dependencies
None